Skip to content

feat: Support Union type in approx_distinct#23714

Merged
Jefffrey merged 3 commits into
apache:mainfrom
mkleen:union_approx_distinct
Jul 23, 2026
Merged

feat: Support Union type in approx_distinct#23714
Jefffrey merged 3 commits into
apache:mainfrom
mkleen:union_approx_distinct

Conversation

@mkleen

@mkleen mkleen commented Jul 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

  • Support the Arrow type Union for approx_distinct

What changes are included in this PR?

  • Enable HLLAccumulator and HllGroupsAccumulator to support Union
  • Tests

Are these changes tested?

Yes

Are there any user-facing changes?

Yes, approx_distinct supports now Union but no breaking changes.

@github-actions github-actions Bot added the functions Changes to functions implementation label Jul 20, 2026
@mkleen
mkleen marked this pull request as ready for review July 20, 2026 09:18
@codecov-commenter

codecov-commenter commented Jul 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.69%. Comparing base (3b1ce16) to head (9096c52).

Additional details and impacted files 
@@           Coverage Diff           @@
##             main   #23714   +/-   ##
=======================================
  Coverage   80.69%   80.69%           
=======================================
  Files        1088     1088           
  Lines      367766   367813   +47     
  Branches   367766   367813   +47     
=======================================
+ Hits       296781   296820   +39     
- Misses      53276    53279    +3     
- Partials    17709    17714    +5

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Jefffrey
Jefffrey reviewed Jul 20, 2026
View reviewed changes
Comment thread datafusion/functions-aggregate/src/approx_distinct.rs Outdated
}

#[test]
fn union_support_hll_acc_and_group_acc() {

@Jefffrey Jefffrey Jul 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not possible to do via SLT?

@mkleen mkleen Jul 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's tricky via SLT. There is no SQL union type but only union operations in datafusion.

@Jefffrey Jefffrey Jul 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what if we add more rows here:

datafusion/datafusion/sqllogictest/src/test_context.rs

Lines 560 to 594 in 3a2bc0a

fn register_union_table(ctx: &SessionContext) {
let union = UnionArray::try_new(
UnionFields::try_new(
// typeids: 3 for int, 1 for string
vec![3, 1],
vec![
Field::new("int", DataType::Int32, false),
Field::new("string", DataType::Utf8, false),
],
)
.unwrap(),
ScalarBuffer::from(vec![3, 1, 3]),
None,
vec![
Arc::new(Int32Array::from(vec![1, 2, 3])),
Arc::new(StringArray::from(vec![
Some("foo"),
Some("bar"),
Some("baz"),
])),
],
)
.unwrap();
let schema = Schema::new(vec![Field::new(
"union_column",
union.data_type().clone(),
false,
)]);
let batch =
RecordBatch::try_new(Arc::new(schema.clone()), vec![Arc::new(union)]).unwrap();
ctx.register_batch("union_table", batch).unwrap();
}

then can use in slts like here

datafusion/datafusion/sqllogictest/test_files/union_function.slt

Lines 18 to 30 in 3a2bc0a

# Note: union_table is registered via Rust code in the sqllogictest test harness
# because there is no way to create a union type in SQL today
##########
## UNION DataType Tests
##########
query ?I
select union_column, union_extract(union_column, 'int') from union_table;
----
{int=1} 1
{string=bar} NULL
{int=3} 3

@mkleen mkleen Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea!

@mkleen
mkleen force-pushed the union_approx_distinct branch from 9096c52 to addd02c Compare July 22, 2026 11:15
@github-actions github-actions Bot added the sqllogictest SQL Logic Tests (.slt) label Jul 22, 2026
@mkleen
mkleen requested a review from Jefffrey July 22, 2026 12:56
mkleen
mkleen commented Jul 22, 2026
View reviewed changes
Comment thread datafusion/sqllogictest/src/test_context.rs
ctx.register_batch("union_table", batch).unwrap();
}

fn register_approx_distinct_union_table(ctx: &SessionContext) {

@mkleen mkleen Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a separate batch which fits better for testing.

@Jefffrey
Jefffrey added this pull request to the merge queue Jul 23, 2026
@Jefffrey

Jefffrey commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

thanks @mkleen

@mkleen

mkleen commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

@Jefffrey thanks for the review!

Merged via the queue into apache:main with commit 1c8295c Jul 23, 2026
38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Jefffrey Jefffrey Jefffrey approved these changes

Assignees

No one assigned

Labels

functions Changes to functions implementation sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@mkleen @codecov-commenter @Jefffrey